-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8366178: Implement JEP 526: Lazy Constants (Second Preview) #27605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| * This is only possible if there is a direct reference from a {@code static final} field | ||
| * to a lazy constant or if there is a chain from a {@code static final} field -- via one | ||
| * or more <em>trusted fields</em> (i.e., {@code static final} fields, | ||
| * {@linkplain Record record} fields, lazy constants, lazy lists, lazy maps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lazy constants and lazy lists/maps are not themselves "trusted final fields" (at least not yet), so not sure they belong here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @Stable annotation makes fields with non‑null values into trusted fields when the class is loaded by a platform class loader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There could be a chain: static final -> record component -> lazy list element -> record component, for example, and this would be a trusted chain. There are other constucts that could be in the chain as well and that are not mentioned (e.g. a List.of()) so perhaps we should do some change here. Easiest is to just remove the mention of lazy*.
|
After some discussion, we have concluded that we need to rework how For
|
| * The returned lazy list strongly references its computing | ||
| * function used to compute elements only so long as there are uncomputed elements | ||
| * after which the computing function is not strongly referenced | ||
| * anymore and may be collected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's okay to say that the computing function is kept strongly reachable until at least all elements have been computed. I'm less sure about the second part being normative text and wonder if it would be better to move that part to an implNote.
| } | ||
|
|
||
| /** | ||
| * {@return a new lazily computed list with the provided {@code size}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor comment here is that "of the given size" might be a bit clearer than "with the provided size".
Implement JEP 526: Lazy Constants (Second Preview)
The lazy list/map implementations are broken out from
ImmutableCollectionsto a separate class.The old benchmarks are not moved/renamed to allow comparison with previous releases.
java.util.Optionalis updated so that its field is annotated with@Stable. This is to allowOptionalinstances to be held in lazy constants and still provide constant folding.Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27605/head:pull/27605$ git checkout pull/27605Update a local copy of the PR:
$ git checkout pull/27605$ git pull https://git.openjdk.org/jdk.git pull/27605/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27605View PR using the GUI difftool:
$ git pr show -t 27605Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27605.diff
Using Webrev
Link to Webrev Comment